[scroll-to-text-fragment] Replace BroadcastChannel with WPT Stash BroadcastChannel was not supported on all platforms, and also caused some flakiness. This patch replaces BroadcastChannel usage with WPT Stash. Also adds a META.yml. Bug: https://github.com/web-platform-tests/wpt/issues/20269 Change-Id: I25f7fa30da59c156de88ee5f7259152369493629 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1925428 Commit-Queue: Nick Burris <nburris@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#723903} diff --git a/scroll-to-text-fragment/META.yml b/scroll-to-text-fragment/META.yml new file mode 100644 index 0000000..13225f6 --- /dev/null +++ b/scroll-to-text-fragment/META.yml
@@ -0,0 +1,4 @@ +spec: https://wicg.github.io/ScrollToTextFragment/ +suggested_reviewers: + - nburris + - bokan
diff --git a/scroll-to-text-fragment/scroll-to-text-fragment-security.html b/scroll-to-text-fragment/scroll-to-text-fragment-security.html new file mode 100644 index 0000000..e253fe9 --- /dev/null +++ b/scroll-to-text-fragment/scroll-to-text-fragment-security.html
@@ -0,0 +1,78 @@ +<!doctype html> +<title>Navigating to a text fragment directive</title> +<meta charset=utf-8> +<link rel="help" href="https://wicg.github.io/ScrollToTextFragment/"> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<script src="/resources/testdriver.js"></script> +<script src="/resources/testdriver-vendor.js"></script> +<script src="/common/utils.js"></script> +<script src="stash.js"></script> +<script> +// Test security restriction for user activation +for (let user_activation of [true, false]) { + promise_test(t => new Promise((resolve, reject) => { + let key = token(); + + if (user_activation) { + test_driver.bless('Open a URL with a text fragment directive', () => { + window.open(`scroll-to-text-fragment-target.html?key=${key}#:~:text=test`, '_blank', 'noopener'); + }); + } else { + window.open(`scroll-to-text-fragment-target.html?key=${key}#:~:text=test`, '_blank', 'noopener'); + } + + fetchResults(key, resolve, reject); + }).then(data => { + assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); + + if (user_activation) { + assert_equals(data.scrollPosition, 'text', 'Expected window.open() with a user activation to scroll to text.'); + } else { + assert_equals(data.scrollPosition, 'top', 'Expected window.open() with no user activation to not activate text fragment directive.'); + } + }), `Test that a text fragment directive requires a user activation (user_activation=${user_activation}).`); +} + +// Test security restriction for no window opener +for (let noopener of [true, false]) { + promise_test(t => new Promise((resolve, reject) => { + let key = token(); + + test_driver.bless('Open a URL with a text fragment directive', () => { + if (noopener) { + window.open(`scroll-to-text-fragment-target.html?key=${key}#:~:text=test`, '_blank', 'noopener'); + } else { + window.open(`scroll-to-text-fragment-target.html?key=${key}#:~:text=test`, '_blank'); + } + }); + + fetchResults(key, resolve, reject); + }).then(data => { + assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); + + if (noopener) { + assert_equals(data.scrollPosition, 'text', 'Expected window.open() with noopener to scroll to text.'); + } else { + assert_equals(data.scrollPosition, 'top', 'Expected window.open() with opener to not activate text fragment directive.'); + } + }), `Test that a text fragment directive is not activated when there is a window opener (noopener=${noopener}).`); +} + +// Test security restriction for no activation in an iframe +promise_test(t => new Promise((resolve, reject) => { + let key = token(); + + let frame = document.createElement('iframe'); + document.body.appendChild(frame); + + test_driver.bless('Navigate the iframe with a text fragment directive', () => { + frame.src = `scroll-to-text-fragment-target.html?key=${key}#:~:text=test`; + }); + + fetchResults(key, resolve, reject); +}).then(data => { + assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); + assert_equals(data.scrollPosition, 'top', 'Expected iframe navigation to not activate text fragment directive.'); +}), 'Test that a text fragment directive is not activated within an iframe.'); +</script> diff --git a/scroll-to-text-fragment/scroll-to-text-fragment-target.html b/scroll-to-text-fragment/scroll-to-text-fragment-target.html index 1595d8b..0c59d8d 100644 --- a/scroll-to-text-fragment/scroll-to-text-fragment-target.html +++ b/scroll-to-text-fragment/scroll-to-text-fragment-target.html
@@ -1,5 +1,6 @@ <!doctype html> <title>Navigating to a text fragment anchor</title> +<script src="stash.js"></script> <script> function isInView(element) { let rect = element.getBoundingClientRect(); @@ -8,8 +9,6 @@ } function checkScroll() { - let bc = new BroadcastChannel('scroll-to-text-fragment'); - let position = 'unknown'; if (window.scrollY == 0) position = 'top'; @@ -30,9 +29,10 @@ else if (isInView(document.getElementById('horizontal-scroll')) && window.scrollX > 0) position = 'horizontal-scroll'; - bc.postMessage({ scrollPosition: position, href: window.location.href }); - bc.close(); - window.close(); + let results = { scrollPosition: position, href: window.location.href }; + + let key = (new URL(document.location)).searchParams.get("key"); + stashResults(key, results); } </script> <style> diff --git a/scroll-to-text-fragment/scroll-to-text-fragment.html b/scroll-to-text-fragment/scroll-to-text-fragment.html index c0018ed..2a94033 100644 --- a/scroll-to-text-fragment/scroll-to-text-fragment.html +++ b/scroll-to-text-fragment/scroll-to-text-fragment.html
@@ -7,6 +7,16 @@ <script src="/resources/testharnessreport.js"></script> <script src="/resources/testdriver.js"></script> <script src="/resources/testdriver-vendor.js"></script> +<script src="/common/utils.js"></script> +<script src="stash.js"></script> +<!-- + This test suite performs scroll to text navigations to + scroll-to-text-fragment-target.html and then checks the results, which are + communicated back from the target page via the WPT Stash server (see stash.py). + This structure is necessary because scroll to text security restrictions + specifically restrict the navigator from being able to observe the result of + the navigation, e.g. the target page cannot have a window opener. +--> <script> let test_cases = [ // Test non-text fragment directives @@ -217,82 +227,18 @@ ]; for (const test_case of test_cases) { - promise_test(t => new Promise(resolve => { - let channel = new BroadcastChannel('scroll-to-text-fragment'); - channel.addEventListener("message", e => { - resolve(e.data); - }, {once: true}); + promise_test(t => new Promise((resolve, reject) => { + let key = token(); test_driver.bless('Open a URL with a text fragment directive', () => { - window.open('scroll-to-text-fragment-target.html' + test_case.fragment, '_blank', 'noopener'); + window.open(`scroll-to-text-fragment-target.html?key=${key}${test_case.fragment}`, '_blank', 'noopener'); }); + + fetchResults(key, resolve, reject); }).then(data => { assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); assert_equals(data.scrollPosition, test_case.expect_position, `Expected ${test_case.fragment} (${test_case.description}) to scroll to ${test_case.expect_position}.`); }), `Test navigation with fragment: ${test_case.description}.`); } - -promise_test(t => new Promise(resolve => { - let channel = new BroadcastChannel('scroll-to-text-fragment'); - channel.addEventListener("message", e => { - assert_equals(e.data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); - - // The first navigation has no user activation. - assert_equals(e.data.scrollPosition, 'top', 'Expected window.open() with no user activation to not activate text fragment directive.'); - - // Now ensure that a navigation with a user activation does activate the text fragment directive. - test_driver.bless('Open a URL with a text fragment directive', () => { - window.open('scroll-to-text-fragment-target.html#:~:text=test', '_blank', 'noopener'); - }); - channel.addEventListener("message", e => { - resolve(e.data.scrollPosition); - }, {once: true}); - }, {once: true}); - - window.open('scroll-to-text-fragment-target.html#:~:text=test', '_blank', 'noopener'); -}).then(scrollPosition => { - assert_equals(scrollPosition, 'text', 'Expected window.open() with a user activation to scroll to text.'); -}), 'Test that a text fragment directive is not activated without a user activation'); - -promise_test(t => new Promise(resolve => { - let channel = new BroadcastChannel('scroll-to-text-fragment'); - channel.addEventListener("message", e => { - assert_equals(e.data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); - - // The first navigation has an opener. - assert_equals(e.data.scrollPosition, 'top', 'Expected window.open() with opener to not activate text fragment directive.'); - - // Now ensure that a navigation with noopener does activate the text fragment directive. - test_driver.bless('Open a URL with a text fragment directive', () => { - window.open('scroll-to-text-fragment-target.html#:~:text=test', '_blank', 'noopener'); - }); - channel.addEventListener("message", e => { - resolve(e.data.scrollPosition); - }, {once: true}); - }, {once: true}); - - test_driver.bless('Open a URL with a text fragment directive', () => { - window.open('scroll-to-text-fragment-target.html#:~:text=test', '_blank'); - }); -}).then(scrollPosition => { - assert_equals(scrollPosition, 'text', 'Expected window.open() with noopener to scroll to text.'); -}), 'Test that a text fragment directive is not activated when there is a window opener.'); - -promise_test(t => new Promise(resolve => { - let channel = new BroadcastChannel('scroll-to-text-fragment'); - channel.addEventListener("message", e => { - resolve(e.data); - }, {once: true}); - - let frame = document.createElement('iframe'); - document.body.appendChild(frame); - - test_driver.bless('Navigate the iframe with a text fragment directive', () => { - frame.src = 'scroll-to-text-fragment-target.html#:~:text=test'; - }); -}).then(data => { - assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.'); - assert_equals(data.scrollPosition, 'top', 'Expected iframe navigation to not activate text fragment directive.'); -}), 'Test that a text fragment directive is not activated within an iframe.'); </script> diff --git a/scroll-to-text-fragment/stash.js b/scroll-to-text-fragment/stash.js new file mode 100644 index 0000000..b3d9ea8 --- /dev/null +++ b/scroll-to-text-fragment/stash.js
@@ -0,0 +1,23 @@ +// Put test results into Stash +function stashResults(key, results) { + fetch(`/scroll-to-text-fragment/stash.py?key=${key}`, { + method: 'POST', + body: JSON.stringify(results) + }); +} + +// Fetch test results from the Stash +function fetchResults(key, resolve, reject) { + fetch(`/scroll-to-text-fragment/stash.py?key=${key}`).then(response => { + return response.text(); + }).then(text => { + if (text) { + try { + const results = JSON.parse(text); + resolve(results); + } catch(e) { + reject(); + } + } + }); +} diff --git a/scroll-to-text-fragment/stash.py b/scroll-to-text-fragment/stash.py new file mode 100644 index 0000000..4956739 --- /dev/null +++ b/scroll-to-text-fragment/stash.py
@@ -0,0 +1,17 @@ +import time + +def main(request, response): + key = request.GET.first("key") + + if request.method == "POST": + # Received result data from target page + request.server.stash.put(key, request.body, '/scroll-to-text-fragment/') + return "ok" + else: + # Request for result data from test page + value = request.server.stash.take(key, '/scroll-to-text-fragment/') + # Poll until data is stashed + while value is None: + time.sleep(.1) + value = request.server.stash.take(key, '/scroll-to-text-fragment/') + return value 